Skip to content

fix(logging): handle WebSocketResponsesRequest in convertToProcessedStreamResponse#3019

Closed
thiscantbeserious wants to merge 1 commit intomaximhq:mainfrom
thiscantbeserious:fix/3000-logging-ws-responses-case
Closed

fix(logging): handle WebSocketResponsesRequest in convertToProcessedStreamResponse#3019
thiscantbeserious wants to merge 1 commit intomaximhq:mainfrom
thiscantbeserious:fix/3000-logging-ws-responses-case

Conversation

@thiscantbeserious
Copy link
Copy Markdown

Summary

convertToProcessedStreamResponse in plugins/logging/utils.go has a switch on requestType that covers six request types but is missing a case for schemas.WebSocketResponsesRequest. Requests routed through the native-WS Responses path fall through to default and get stored under streaming.StreamTypeChat instead of streaming.StreamTypeResponses. This mis-labels the accumulated chunk data in the log store, causing downstream consumers that branch on stream type to receive incorrectly shaped content for WebSocket Responses streams.

Root cause: schemas.WebSocketResponsesRequest was added after the switch was written and the case was never added.

Changes

  • plugins/logging/utils.go: extend the ResponsesStreamRequest case to also match schemas.WebSocketResponsesRequest, routing it to streaming.StreamTypeResponses.
  • plugins/logging/utils_test.go: add five unit tests covering the new case, a regression guard for the existing ResponsesStreamRequest arm, a parity assertion between both types, nil-input safety, and the default fallback.

Type of change

  • Bug fix

Affected areas

  • Plugins

How to test

go build ./plugins/logging/...
go test ./plugins/logging/... -cover -run TestConvertToProcessed
go vet ./plugins/logging/...

Expected: all five TestConvertToProcessed* tests pass. WebSocketResponsesRequest and ResponsesStreamRequest both resolve to streaming.StreamTypeResponses.

Runtime validation requires #2997 and #2999 to land first (those PRs restore the WS log row write path on main). Once they are in, the steps from the issue reproduce the correct stream_type = responses in the stored row.

Screenshots/Recordings

N/A (logging internals, no UI change)

Breaking changes

  • No

Related issues

Closes #3000

Related: #2997 and #2999 are prerequisites for runtime observation of this defect (they restore the WS log row write path on main). This bug was discovered while working on PR #2775 in thiscantbeserious/bifrost. That branch contains the fix extracted here as a side effect of the feature work.

Security considerations

None. This change only affects how an existing log accumulation result is categorised by stream type.

Checklist

  • I read docs/contributing/README.md and followed the guidelines
  • I added/updated tests where appropriate
  • I updated documentation where needed
  • I verified builds succeed (Go and UI)
  • I verified the CI pipeline passes locally if applicable

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 24, 2026

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: c70f9172-28d0-4ba8-a658-f3cd7456c628

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@thiscantbeserious
Copy link
Copy Markdown
Author

Superseded by #3018 (merged 2026-04-24), which bundles the fix for this issue and several other native WS reliability bugs. Closing this draft as redundant.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: convertToProcessedStreamResponse lacks a case for WebSocketResponsesRequest, routes it as chat

1 participant